Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add enable option globally #124

Merged
merged 2 commits into from
Apr 22, 2024
Merged

feat: add enable option globally #124

merged 2 commits into from
Apr 22, 2024

Conversation

drupol
Copy link
Contributor

@drupol drupol commented Apr 17, 2024

This PR:

I won't be able to test this until late tonight, help needed!

Copy link
Member

@isabelroses isabelroses left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not put it in modules/home-manager/globals.nix.

And I don't belive you need the flavour = lib.mkDefault cfg.flavour; because of the aforementioned globals.

@drupol
Copy link
Contributor Author

drupol commented Apr 17, 2024

doing it

@drupol
Copy link
Contributor Author

drupol commented Apr 17, 2024

Just tested on my own local config with:

{ inputs, ... }:

{
  imports = [
    inputs.catppuccin.homeManagerModules.catppuccin
  ];

  catppuccin = {
    enable = true;
    flavour = "latte";
  };
}

And that works :)

@drupol drupol marked this pull request as ready for review April 17, 2024 16:16
@netlooker
Copy link

Many thanks @drupol as I was pulling my hair out to make it work 🙏

@drupol
Copy link
Contributor Author

drupol commented Apr 17, 2024

@isabelroses Anything else left to do in here?

@isabelroses
Copy link
Member

isabelroses commented Apr 17, 2024

@isabelroses Anything else left to do in here?

I don't think you need to add flavour = lib.mkDefault cfg.flavour; to each config.programs.<name>.catppuccin though since this already is handled by another global.

Other then that in my opinion its good but ultimately its up to @getchoo.

@drupol
Copy link
Contributor Author

drupol commented Apr 17, 2024

I just fixed it.

@drupol
Copy link
Contributor Author

drupol commented Apr 18, 2024

I'm using nixfmt-rfc-style but going to fix the PR with your style just now.

Copy link
Member

@getchoo getchoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for getting the ball rolling here btw :)

modules/home-manager/globals.nix Outdated Show resolved Hide resolved
modules/home-manager/globals.nix Outdated Show resolved Hide resolved
modules/home-manager/globals.nix Show resolved Hide resolved
@drupol
Copy link
Contributor Author

drupol commented Apr 18, 2024

I updated the PR, WDYT ?

modules/home-manager/alacritty.nix Outdated Show resolved Hide resolved
@isabelroses
Copy link
Member

Probably should rename PR to feat: enable all option

docs/home-manager-options.md Outdated Show resolved Hide resolved
modules/home-manager/globals.nix Outdated Show resolved Hide resolved
modules/lib/default.nix Outdated Show resolved Hide resolved
modules/nixos/globals.nix Outdated Show resolved Hide resolved
modules/nixos/globals.nix Outdated Show resolved Hide resolved
@drupol drupol changed the title feat: add home-manager module feat: add enable option globally Apr 18, 2024
@drupol
Copy link
Contributor Author

drupol commented Apr 20, 2024

Anything else to do?

Copy link
Contributor

@pluiedev pluiedev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one small thing

modules/home-manager/globals.nix Outdated Show resolved Hide resolved
Copy link
Member

@getchoo getchoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@getchoo getchoo merged commit e45a44e into catppuccin:main Apr 22, 2024
2 checks passed
@becknik
Copy link

becknik commented Apr 22, 2024

Thank you so much! This really feels like a enhancement to this project
I think documenting it in the README would be nice tho

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add global enableAll setting
6 participants